[Server] Bind Streamable HTTP sessions to the principal that opened them by nicolas-grekas · Pull Request #627 · modelcontextprotocol/php-sdk

nicolas-grekas · 2026-10-27T10:16:08Z

Fixes #529

A handshake-era session now only serves the principal that opened it, as the security best practices recommend. When another principal sends the session id, the server answers 404 like for an unknown session, and its DELETE doesn't end the session.

I found this while building an MCP server for the Symfony showcase app: carol could call tools on the session alice opened.

On the open questions of #529:

The binding always applies. Clients keep using their own token, and a 404 makes them re-initialize, so I see nothing to opt in to.
It's in Protocol, next to the other session checks, so custom stores don't need to know about it.
A token without sub identifies no principal. An app that authenticates requests itself sets the new StreamableHttpTransport::PRINCIPAL_ATTRIBUTE request attribute.

AuthorizationMiddleware builds the principal from iss, sub and client_id/azp, like #625 does for requestState. Protocol only gets an opaque string and stores its SHA-256 in the session.

McpBundle (symfony/ai) authenticates with Symfony Security, not with AuthorizationMiddleware. It'll need to set this attribute from the user identifier for the binding to work there.

Copilot

🟡 Changes recommended

A session-store read failure can bypass ownership checks, allowing a caller with a valid subjectless token to delete another principal’s session.

3 open issues

What changed in this PR

This PR adds principal binding to handshake-era Streamable HTTP sessions, addressing #529 within Protocol without requiring custom session stores to understand authentication.

Changes:

Derives a stable principal from token claims and passes it through transport callbacks.
Stores a principal fingerprint and checks ownership before session use or termination.
Adds binding tests and documents external authentication integration and compatibility changes.

File	Description
`tests/Unit/Server/Transport/SessionBindingTest.php`	Adds session ownership, token refresh, and deletion tests.
`tests/Unit/Server/Transport/Http/Middleware/AuthorizationMiddlewareTest.php`	Tests principal derivation from token claims.
`tests/Unit/Server/Transport/AccessTokenFlowTest.php`	Adapts token-flow tests to principal-bound sessions.
`tests/Unit/Server/BuilderTest.php`	Updates expected session reads during termination.
`src/Server/Transport/TransportInterface.php`	Documents principal arguments for callbacks.
`src/Server/Transport/StreamableHttpTransport.php`	Reads the principal attribute for POST and DELETE requests.
`src/Server/Transport/ManagesTransportCallbacks.php`	Updates callback type annotations.
`src/Server/Transport/Http/Middleware/AuthorizationMiddleware.php`	Derives and attaches the authenticated principal.
`src/Server/Transport/BaseTransport.php`	Forwards principals to transport listeners.
`src/Server/Protocol.php`	Stores fingerprints and checks session ownership.
`docs/run/authorization.md`	Explains binding and application-provided principals.
`CHANGELOG.md`	Records binding behavior and callback signature changes.

_{Review effort: Balanced}

chr-hertel · 2026-11-08T11:49:20Z

Thanks @nicolas-grekas - rebased this onto main and added four commits on top, since there were cases where the binding silently didn't apply. Feel free to squash or rework them, WDYT?

endSession() now passes on the bool from [Server] Answer a DELETE of an unknown or ended session with 404 #638, so a DELETE of another principal's session gets the same 404 as an unknown one - that's in the first commit
50be587: the transport now gets the principal from the new AccessToken::getPrincipal(), so AuthorizationMiddleware doesn't set the attribute anymore - before, an app that only set AccessToken::class had no binding at all
Same commit: tokens without sub now bind to issuer & client instead of all sharing null. I didn't go for the fail closed of [Server] Bind requestState to the request it was minted for #625, since the owner couldn't use the session then. Tokens of one client still share sessions unless the app sets PRINCIPAL_ATTRIBUTE
Also there: an invalid PRINCIPAL_ATTRIBUTE now gets a 500 - an integer user id used to end up as null, so user 7 was served on the session of user 42
406ae6a: a failed session read or a store outage now gets a 503 instead of sending the owner to re-initialize, and a DELETE doesn't end the session then. Stdio & in-memory log a store outage at shutdown instead of throwing
999c304: getPrincipal() uses serialize(), so a claim that isn't UTF-8 can't make it throw
aa88e32: malformed JSON and an initialize with a session id now get a 400, and a failure before the session is checked a 500, instead of a 200 - those answers used to carry and remove the messages queued for the session in the header, also for another principal

Not perfect yet:

during a failed read, another principal gets the 503 as well, so it can tell the session exists
session data that decodes to nothing gets the 503 until its TTL is over

Unit, integration & interop suites, phpstan and php-cs-fixer pass locally - didn't run conformance. The description needs an update as well: it still says that a token without sub identifies no principal and that AuthorizationMiddleware builds the principal.

Copilot

🟡 Changes recommended

A non-UTF-8 identity claim can make getPrincipal() throw outside the exception guard, causing POST and DELETE to fail without a controlled HTTP error response.

1 open issue

Handle invalid UTF-8 identity claims without throwing from POST or DELETE · New

3 resolved since last review

_{Review effort: Balanced}

Copilot

🔵 Needs a closer look

Error responses generated before session ownership validation can expose and drain another principal’s queued notifications, creating a critical cross-user data leak.

No open issues 🎉

1 previously missed issue

In code that hasn't changed since last review

Prevent rejected requests from draining another principal’s queue

src/Server/Transport/StreamableHttpTransport.php:223

A caller authenticated as Carol can POST malformed JSON or an initialize request with Alice's session ID: both errors are returned before ownership validation, but createJsonResponse() then drains Alice's outgoing queue and exposes her queued notifications as SSE. Only retrieve queued messages after successful principal validation, or route these pre-validation errors through the sessionless immediate-response path. Add HTTP regression tests verifying that both rejected requests neither expose nor consume another principal's queue.

1 resolved since last review

Handle invalid UTF-8 identity claims without throwing from POST or DELETE

_{Review effort: Balanced}

A session opened without a principal now records it, so a session the store fails to read serves no one. Who opened a session is checked before waiting for its lock, so another principal can't tell the session is busy, and an initialize never takes the lock.

Bind tokens without a subject to their issuer and client, and refuse an invalid principal attribute.

A failed read no longer sends the owner to re-initialize, and a store outage no longer escapes from a DELETE or a transport's shutdown.

…cipal

nicolas-grekas requested review from CodeWithKyrian, Nyholm, chr-hertel and soyuka as code owners October 27, 2026 10:16

chr-hertel added Server Issues & PRs related to the Server component breaking change Breaking the Backwards Compatibility Promise auth Issues and PRs related to Authentication / OAuth bug Something isn't working labels Oct 27, 2026

chr-hertel requested a balanced review from Copilot October 27, 2026 23:41

Copilot started reviewing on behalf of chr-hertel October 27, 2026 23:42

View session

Copilot AI reviewed Oct 27, 2026

View reviewed changes

Comment thread src/Server/Protocol.php Resolved

Comment thread src/Server/Protocol.php Resolved

Comment thread src/Server/Transport/StreamableHttpTransport.php Resolved

nicolas-grekas mentioned this pull request Oct 28, 2026

[Server] Answer a DELETE of an unknown or ended session with 404 #638

Merged

chr-hertel added this to the 0.10.0 milestone Nov 4, 2026

chr-hertel force-pushed the session-principal branch from 3c8363a to b9754c1 Compare November 8, 2026 11:49

chr-hertel force-pushed the session-principal branch from b9754c1 to 05be8cf Compare November 8, 2026 12:16

chr-hertel requested a balanced review from Copilot November 8, 2026 12:22

Copilot started reviewing on behalf of chr-hertel November 8, 2026 12:22

View session

Copilot AI reviewed Nov 8, 2026

View reviewed changes

Comment thread src/Server/Transport/StreamableHttpTransport.php Resolved

chr-hertel requested a balanced review from Copilot November 8, 2026 12:43

Copilot started reviewing on behalf of chr-hertel November 8, 2026 12:44

View session

Copilot AI reviewed Nov 8, 2026

View reviewed changes

nicolas-grekas and others added 6 commits November 8, 2026 13:52

[Server] Bind Streamable HTTP sessions to the principal that opened them

e9a5848

[Server] Derive the session principal from the access token

50be587

Bind tokens without a subject to their issuer and client, and refuse an invalid principal attribute.

[Server] Answer 503 when the session owner cannot be read

406ae6a

A failed read no longer sends the owner to re-initialize, and a store outage no longer escapes from a DELETE or a transport's shutdown.

[Server] Stop a claim that is not UTF-8 from failing the session prin…

999c304

…cipal

[Server] Stop refused requests from draining another principal's queue

aa88e32

chr-hertel force-pushed the session-principal branch from 73b74b7 to aa88e32 Compare November 8, 2026 12:58

chr-hertel approved these changes Nov 8, 2026

View reviewed changes

chr-hertel merged commit 9604e0e into modelcontextprotocol:main Nov 8, 2026
31 checks passed

chr-hertel mentioned this pull request Nov 8, 2026

[Server] Answer 404 for a session that a handler cleared or that holds corrupt data #682

Merged

Navigation Menu

Repository navigation

[Server] Bind Streamable HTTP sessions to the principal that opened them - #627

[Server] Bind Streamable HTTP sessions to the principal that opened them#627
chr-hertel merged 6 commits into
modelcontextprotocol:mainfrom
nicolas-grekas:session-principal

nicolas-grekas commented Oct 27, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chr-hertel commented Nov 8, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

nicolas-grekas commented Oct 27, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chr-hertel commented Nov 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

chr-hertel commented Nov 8, 2026 •

edited

Loading